Conversation
ghost
added
the
| return new SocketsHttpHandler(); | ||
| return new SocketsHttpHandler() | ||
| { | ||
| AllowAutoRedirect = false | ||
| }; | ||
| #else | ||
| return new HttpClientHandler(); | ||
| return new HttpClientHandler() | ||
| { | ||
| AllowAutoRedirect = false | ||
| }; |
There was a problem hiding this comment.
nit:
#if NETCOREAPP
return new SocketsHttpHandler()
#else
return new HttpClientHandler();
#endif
{
AllowAutoRedirect = false
};| // Clear the authorization header. | ||
| request.Headers.Remove(HttpHeader.Names.Authorization); | ||
|
|
||
| if (AzureCoreEventSource.Singleton.IsEnabled()) |
There was a problem hiding this comment.
Just curious why we don't do these checks in the eventSource instead?
There was a problem hiding this comment.
To avoid converting Uris to strings if the ES is disabled.
|
After this update, I assume I should remove my changes in Communications, correct? |
Correct. You would also need to add a ProjectReference to Azure.Core (to get the new feature) and flip it back to PackageReference when Azure.Core ships with the fix. |
Can we just wait for the package? |
That works too! |
Co-authored-by: Christopher Scott <chriscott@hotmail.com>
| response.Dispose(); | ||
|
|
||
| // Clear the authorization header. | ||
| request.Headers.Remove(HttpHeader.Names.Authorization); |
There was a problem hiding this comment.
dotnet/runtime#26475 (comment)
TLDR to prevent accidental secret leaks.
4 participants
Fixes: #22746